Replace isomorphic-dompurify with dompurify, drop jsdom from production deps#808
Replace isomorphic-dompurify with dompurify, drop jsdom from production deps#808brendanlong wants to merge 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces isomorphic-dompurify and jsdom with the standard dompurify package. However, this change introduces a critical issue for Server-Side Rendering (SSR) in Next.js. Since dompurify requires a DOM implementation to function, it will cause a runtime crash on the server where window is undefined. Furthermore, failing to sanitize content on the server side creates a Cross-Site Scripting (XSS) vulnerability in the initial HTML payload before hydration occurs.
| import { useEffect, useRef, useMemo } from "react"; | ||
| import { useHotkeys } from "react-hotkeys-hook"; | ||
| import DOMPurify from "isomorphic-dompurify"; | ||
| import DOMPurify from "dompurify"; |
There was a problem hiding this comment.
Replacing isomorphic-dompurify with dompurify while removing jsdom from production dependencies will cause a runtime crash during Server-Side Rendering (SSR).
Contrary to the assumption in the PR description, "use client" components in Next.js are still executed on the server to generate the initial HTML payload. Standard dompurify requires a DOM implementation (like window.document) to function. In a Node.js environment (SSR), the dompurify import returns a factory function rather than an initialized instance. Consequently, calling DOMPurify.addHook at the module level (line 18) or DOMPurify.sanitize within useMemo (lines 277, 285) will throw a TypeError because these methods do not exist on the factory function.
Furthermore, if you were to guard these calls with typeof window !== 'undefined', you would be sending unsanitized HTML to the browser in the initial payload. This introduces a Cross-Site Scripting (XSS) vulnerability, as the browser will parse and potentially execute malicious scripts before React hydration occurs. To maintain SSR support and security, you should continue using isomorphic-dompurify or manually provide a DOM implementation (like jsdom or linkedom) to the dompurify factory on the server.
There was a problem hiding this comment.
Thanks for the review! I investigated each concern:
1. "Runtime crash" / "factory function" — This is inaccurate. DOMPurify v3's default export is createDOMPurify() (the result, not the factory). When there's no window, it returns a stub object with isSupported = false and sanitize() returns the input unchanged rather than throwing. See purify.es.mjs L305-314. The addHook call is already guarded with if (DOMPurify.isSupported) (line 20).
2. XSS from unsanitized SSR HTML — Valid concern in general, but doesn't apply here:
EntryContentwraps this component in a<Suspense>boundary and usesuseSuspenseQuery(EntryContent.tsx L78), soEntryContentBodynever renders during SSR — the fallback is shown instead- Feed content is also cleaned server-side during the feed processing pipeline before it reaches the database
So there's no path where unsanitized HTML ends up in the SSR payload.
— Claude
1dbbe1d to
90067fd
Compare
…on deps (#804) Since DOMPurify is only used in a "use client" component (EntryContentBody), it always runs in the browser where native DOM is available. This means we don't need isomorphic-dompurify's jsdom-based server fallback. - Replace `isomorphic-dompurify` with `dompurify` (uses native browser DOM) - Remove `jsdom` from production dependencies (remains available via vitest) - Reduces transitive dependency surface area (no more undici in production) Closes #804 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
90067fd to
3497595
Compare
Summary
isomorphic-dompurifywithdompurify— since DOMPurify is only used inEntryContentBody.tsx(a"use client"component), it always runs in the browser where native DOM is availablejsdomfrom production dependencies (it remains available via vitest for tests)undicifrom production (the source of 12 recent security vulnerabilities fixed in Fix 12 Dependabot security vulnerabilities (undici) #803)Closes #804
Test plan
🤖 Generated with Claude Code